Code review

Overview. The core mechanism is sound — react-hotkeys-hook 4.5.0 matches the new tokens (bracketleft, bracketright, period) via mapKey(event.code), so the headline fix (brackets working on non-US layouts) genuinely works, and the fake-timers rewrite of navigation-api.test.ts is strictly tighter than the old real-timer assertion with no coverage loss. However, review found real regressions for keyboards where punctuation glyphs live on different physical keys (AltGr layouts), plus several design-level issues. Findings 1–3 share one root cause: glyph→token rewriting that assumes the glyph came from the US-layout physical key.

Findings (most severe first)

1. hotkeyStrings.ts:59 — normalizeHotkeyKey rewrites punctuation glyphs to physical-key tokens without knowing which physical key produced them, retargeting recorded hotkeys to the wrong key.
   A German QWERTZ user records AltGr+8, which types [ with e.code='Digit8'. Digit8 isn't in HOTKEY_KEY_ALIASES_BY_CODE, so the glyph falls through getHotkeyKeyFromEvent and is then rewritten to bracketleft by normalizeHotkeyKey. The saved hotkey never fires on AltGr+8 (the library sees keyCode '8' / pressedKey '[') and instead fires on the physical BracketLeft key — the ü key. Pre-PR the stored literal [ matched via event.key and worked.

2. useHotkeyData.ts:66 — read-time canonicalization silently rebinds legacy persisted glyph bindings recorded on non-US layouts to a different physical key.
   Same mechanism as Not working on windows ( no readline ) but pyreadline is avalilable #1 on the persistence path: a pre-PR custom [ binding that a QWERTZ user triggered via AltGr+8 is rewritten to bracketleft on every load, so after upgrading, their working chord stops firing and the ü key starts triggering the action — with no notice, since the modal still displays [.

3. hotkeyStrings.ts:90 — canonicalizeHotkeyString misses legacy shifted glyphs (>, <, {, :), so conflict detection misses equivalents and one keystroke fires two hotkeys.
   The pre-PR recorder stored shift+> for Shift+Period. That string survives canonicalization; a post-PR recording of Shift+Period on another action produces shift+period; conflictMap.get('shift+period') can't hit shift+>, so both save, and one Shift+. keydown fires the legacy binding via pressedKey '>' and the new one via keyCode 'period'. Narrow (requires a pre-existing shifted-punctuation custom binding) but fully reproducible.

4. hotkeyStrings.ts:97 — display and modal search use a hardcoded US glyph table, so the exact users this PR targets see keycap labels that don't exist on their keyboard.
   A QWERTZ user sees "Increment tool width: ]" but the triggering keycap is labeled +; ЙЦУКЕН users see [ for the х key; the HotkeysModal search filter matches only the US glyph via platformKeys. navigator.keyboard.getLayoutMap() would resolve the real glyphs as a Chromium-only progressive enhancement, with the US table as fallback.

5. HotkeyListItem.tsx:171 — the recorder unconditionally coerces the 11 punctuation codes to physical tokens, discarding the letter the user typed.
   An AZERTY user pressing M (e.code='Semicolon') records semicolon and the modal displays ; for a keycap labeled M; a Russian user intending to bind the letter х gets bracketleft, which conflicts with the tool-width default and hard-disables the save button. Bindings still fire on the same physical key, so the harm is display accuracy and intent rather than function — and the conflict block is arguably correct since the keys physically collide — but it's worth a deliberate decision.

6. useHotkeyData.ts:66 — legacy bindings are canonicalized on every rebuild forever instead of via the repo's established one-time persist migration.
   hotkeysSlice already has _version: 1 and a wired persistConfig.migrate hook, and upscaleSlice.ts shows the exact v1→v2 convention. A migration mapping customHotkeys values through canonicalizeHotkeyString would retire the per-read shim (the PR checklist's migration box is unchecked).

7. hotkeyStrings.ts:11 — the same 11-key punctuation correspondence is hand-maintained in four tables, and normalizeHotkeyKey allocates a fresh ~17-entry object per call.
   Code→token (HOTKEY_KEY_ALIASES_BY_CODE), token→glyph (HOTKEY_KEY_DISPLAY_ALIASES), plus byte-identical glyph→token blocks in both the mac and non-mac branches of normalizeHotkeyKey. Adding a key (e.g. IntlBackslash) means editing four places, and a missed edit silently breaks record/display for that key. The allocation runs ~294× per buildHotkeysData, which is memoized per component across ~91 useRegisteredHotkeys call sites. Deriving all maps from one {code, token, glyph} table at module scope fixes both.

8. useHotkeyData.ts:105 — physical-key tokens only match via event.code, so bracket hotkeys stop firing for events with an empty code (synthetic new KeyboardEvent('keydown', {key:'['}) from test automation, low-fidelity remote-desktop clients). Pre-PR the literal [ matched via the event.key path. Mainstream real/virtual keyboards populate code, so this is low impact — mostly a test-automation concern.

Verified non-issues

• Numpad punctuation recording produces bindings the numpad can't trigger, but this was equally broken pre-PR (the library maps a registered . to period anyway) — not a regression.
• The IS_MAC_OS move/re-export resolves cleanly with no import cycle, and no other consumer renders raw hotkey strings.
• Reusing the library's useRecordHotkeys instead of the bespoke recorder was considered and rejected: it never emits mod, doesn't remove keys on keyup, and its mapKey isn't safely deep-importable (no runtime dist/parseHotkeys.js).

Suggested direction: apply the glyph→token mapping only when e.code confirms the physical key (i.e., rely on getHotkeyKeyFromEvent and drop the glyph entries from normalizeHotkeyKey's general path), and do legacy canonicalization as a hotkeysSlice v1→v2 persist migration. That addresses findings 1, 2, 3, and 6 together.

🤖 Generated with Claude Code

@lstein

Addressed review feedback by separating physical-key recording from stored glyph normalization.

Punctuation glyphs are no longer rewritten to physical key tokens unless KeyboardEvent.code identifies the physical key, preventing AltGr/non-US custom bindings from being retargeted. Custom hotkeys are no longer canonicalized on read; persisted hotkeys now migrate through hotkeys slice v2. Conflict detection now expands legacy glyph aliases, including shifted punctuation glyphs, without mutating saved user bindings. Physical punctuation display/search can use navigator.keyboard.getLayoutMap() when available.

I thought i posted reply yesterday, but apparently i did not.